Skip to content

Less granular reader packages#7620

Merged
adriansr merged 3 commits intoelastic:masterfrom
kvch:refactor/filebeat/less-granular-readers
Aug 1, 2018
Merged

Less granular reader packages#7620
adriansr merged 3 commits intoelastic:masterfrom
kvch:refactor/filebeat/less-granular-readers

Conversation

@kvch
Copy link
Copy Markdown
Contributor

@kvch kvch commented Jul 17, 2018

I created new packages:

  • readjson: includes JSON and DockerJSON
  • readline: includes LimitReader, StripNewLineReader, etc.

I left multiline as is for now.

@kvch kvch added discuss Issue needs further discussion. review refactoring Filebeat Filebeat labels Jul 17, 2018
@kvch kvch requested a review from urso July 17, 2018 18:59
Comment thread filebeat/reader/readjson/json.go Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type JSONReader should have comment or be unexported

Comment thread filebeat/reader/readjson/json.go Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type JSONReader should have comment or be unexported

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Jul 19, 2018

jenkins test this

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Jul 19, 2018

The failing test on Jenkins is unrelated to this PR.

@kvch kvch requested a review from ph July 31, 2018 08:08
@urso
Copy link
Copy Markdown

urso commented Jul 31, 2018

I like that readjson very much sets expectations on what is supported. How about renaming readline to readfile?

@kvch kvch force-pushed the refactor/filebeat/less-granular-readers branch from fde83c6 to d17e987 Compare August 1, 2018 12:19
@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Aug 1, 2018

It's much better indeed. I've renamed it.

Copy link
Copy Markdown

@urso urso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WFG

@adriansr adriansr merged commit d241f3a into elastic:master Aug 1, 2018
@kvch kvch added the v6.5.0 label Aug 30, 2018
kvch added a commit that referenced this pull request Aug 31, 2018
Cherry-pick of PR #7620 to 6.x branch. Original message:

I created new packages:

    readjson: includes JSON and DockerJSON
    readline: includes LimitReader, StripNewLineReader, etc.

I left multiline as is for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discuss Issue needs further discussion. Filebeat Filebeat refactoring review v6.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants